Repository navigation
Conversation
…(ZTS) Since phpGH-23309 OnUpdateInternedStringsBuffer() rejects any change once OPcache is set up, to catch diverging php_admin_value settings of FPM pools. Under ZTS every thread started after OPcache's startup re-applies the INI entries with ZEND_INI_STAGE_STARTUP into its own, zeroed zend_accel_globals; the handler rejects that as well, logs a warning for every new thread and leaves ZCG(accel_directives). interned_strings_buffer at 0 in that thread (e.g. opcache_get_configuration() reports 0 under FrankenPHP and Apache mod_php with threaded MPMs). Accept the value at ZEND_INI_STAGE_STARTUP: it is the value the shared memory was set up with. Changes at later stages (FPM pools use ZEND_INI_STAGE_ACTIVATE) are still rejected.
|
Prepared with an AI assistant (Claude); the text below is AI-generated (quoted, as asked in CONTRIBUTING.md).
|
ndossche
left a comment
ndossche
left a comment
There was a problem hiding this comment.
Looks good, thanks! Please see my minor nit
| static ZEND_INI_MH(OnUpdateInternedStringsBuffer) | ||
| { | ||
| if (accel_startup_ok) { | ||
| /* Under ZTS every new thread re-applies the startup value (ZEND_INI_STAGE_STARTUP) */ |
There was a problem hiding this comment.
Nit: This should IMO also describe why keeping the value is safe. It should also indicate that NTS does not have this problem because it clears accel_startup_ok
Explain why accepting the value at ZEND_INI_STAGE_STARTUP is safe and why NTS is not affected.
|
Thanks! Addressed in 51df7a0. |
ndossche
left a comment
ndossche
left a comment
There was a problem hiding this comment.
Thanks. I'll keep this open for a bit to see if other reviews come in, and take the merge responsibility upon me.
2 participants
Prepared with an AI assistant (Claude), which also ran the reproduction and the tests on my machine; I reviewed the result. The text below is AI-generated (quoted, as asked in CONTRIBUTING.md).
🤖 Generated with Claude Code